Automated Test: ecosystem-sync-integration-after #317
Conversation
📝 WalkthroughWalkthroughThe changes introduce Changes
Sequence DiagramsequenceDiagram
participant Client
participant GroupAssignee
participant AssignmentSource
participant SyncUtils
participant SyncTask
participant Integration
Client->>GroupAssignee: assign(group, user, assignment_source)
GroupAssignee->>SyncUtils: sync_group_assignee_outbound(group, user, assign=True, assignment_source)
SyncUtils->>AssignmentSource: to_dict() [serialize source]
SyncUtils->>SyncTask: sync_assignee_outbound(..., assignment_source_dict)
SyncTask->>AssignmentSource: from_dict(assignment_source_dict) [deserialize]
SyncTask->>Integration: should_sync(attribute, sync_source)
Integration-->>SyncTask: skip if sync_source.integration_id matches
SyncTask->>Integration: sync_assignee_outbound(..., assignment_source)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/sentry/integrations/mixins/issues.py (1)
410-422:⚠️ Potential issue | 🟡 MinorFix
ExampleIntegration.sync_status_outboundto accept the new parameter.Most implementations (Jira, JiraServer, Vsts) use
**kwargsand are compatible with the newassignment_sourceparameter. However,ExampleIntegration.sync_status_outboundatsrc/sentry/integrations/example/integration.py:161lacks**kwargsand will not accept the new parameter, causing compatibility issues. Add**kwargsto its signature:def sync_status_outbound(self, external_issue, is_resolved, project_id, **kwargs): pass
🤖 Fix all issues with AI agents
In `@src/sentry/integrations/services/assignment_source.py`:
- Around line 14-18: The dataclass AssignmentSource currently sets queued:
datetime = timezone.now() which evaluates at import time; change it to use a
default factory so each instance gets the current time at creation (e.g.,
replace the queued default with field(default_factory=timezone.now)), and import
dataclasses.field if not already imported to support this change.
🧹 Nitpick comments (1)
tests/sentry/integrations/services/test_assignment_source.py (1)
13-19: Typo in test method name:inalid→invalid.- def test_from_dict_inalid_data(self): + def test_from_dict_invalid_data(self):
| @dataclass(frozen=True) | ||
| class AssignmentSource: | ||
| source_name: str | ||
| integration_id: int | ||
| queued: datetime = timezone.now() |
There was a problem hiding this comment.
Bug: timezone.now() is evaluated at class definition time, not instance creation.
All AssignmentSource instances created without an explicit queued value will share the same timestamp (the time the module was loaded). Use field(default_factory=...) instead.
🐛 Proposed fix
-from dataclasses import asdict, dataclass
+from dataclasses import asdict, dataclass, field
from datetime import datetime
from typing import TYPE_CHECKING, Any
from django.utils import timezone
if TYPE_CHECKING:
from sentry.integrations.models import Integration
from sentry.integrations.services.integration import RpcIntegration
`@dataclass`(frozen=True)
class AssignmentSource:
source_name: str
integration_id: int
- queued: datetime = timezone.now()
+ queued: datetime = field(default_factory=timezone.now)🧰 Tools
🪛 Ruff (0.14.14)
[warning] 18-18: Do not perform function call timezone.now in dataclass defaults
(RUF009)
🤖 Prompt for AI Agents
In `@src/sentry/integrations/services/assignment_source.py` around lines 14 - 18,
The dataclass AssignmentSource currently sets queued: datetime = timezone.now()
which evaluates at import time; change it to use a default factory so each
instance gets the current time at creation (e.g., replace the queued default
with field(default_factory=timezone.now)), and import dataclasses.field if not
already imported to support this change.
This pull request was automatically created by
@coderabbitai/e2e-reviewer.Batch created pull request.
Summary by CodeRabbit
New Features
Tests